Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler: use freeze_type as node type if node doesn't have a type #7161

Merged
merged 1 commit into from
Dec 16, 2018

Conversation

asterite
Copy link
Member

@asterite asterite commented Dec 8, 2018

Fixes #7160

When the compiler sees a method return type it will try to resolve it and then set it as the def's @freeze_type. This instance variable holds a type that eventually must match the type that is normally inferred. The compiler doesn't use @freeze_type for anything else.

This PR changes that: if there's no inferred type for a node but it has a @freeze_type, the node's type will be "inferred" to be that type. So if you say a method's return type is Int32, even before the compiler can deduce its real type and see if it matches Int32 it will have an Int32 type. This allows using the method in a recursive method and still be able to fully "infer" it's type (it's a bit of cheating because we are saying the type, but previously this didn't work).

One example is the issue this PR solves:

class Tree
  def initialize(@value : Int32, @children : Array(Tree))
  end

  def sum : Int32
    @value + @children.each.map(&.sum).sum
  end
end

Tree.new(1, [] of Tree).sum

Here the compiler will try to type Tree.new(...).sum and for that it will need to type @children.each.map(&.sum), which needs the type of sum, which isn't computed yet. The compiler before this PR would fail. With this PR it will succeed because we now know the type of sum.

Don't worry: the compiler will later check that the type is effectively what we told it.

An interesting side effect of this change is that in these recursive scenarios the return type isn't a hint anymore, it's a type that's used for inference. For example, this works as well:

class Tree
  def initialize(@value : Int32, @children : Array(Tree))
  end

  def sum : Float64
    @value + @children.each.map(&.sum).sum
  end
end

Tree.new(1, [] of Tree).sum

Because @value is Int32 and sum is told to be Float64, the right-hand side expression @children... is deduced to be Float64, and Int32 + Float64 gives Float64, so this compiles fine. It's a bit spooky, but it's fine :-)

@bcardiff Would it be possible to run the script that checks the compiler works fine for many of the major crystal projects? This is a simple change, all specs pass, but I wonder if it could break something that I'm not seeing.

Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @asterite 👍

P.S: I ran this against Kemal master 💯

@RX14 RX14 added this to the 0.27.1 milestone Dec 16, 2018
@RX14 RX14 merged commit dfe7cf8 into crystal-lang:master Dec 16, 2018
@asterite asterite deleted the feature/use-freeze-type branch March 30, 2019 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't infer block return type even though there is nothing to infer
3 participants